Merged
Conversation
…into feat/#202-terms # Conflicts: # src/main/java/org/sopt/kareer/domain/member/controller/MemberController.java
hyomee2
requested review from
eraser502 and
jeong1112
and removed request for
eraser502
📝 WalkthroughWalkthrough약관 도메인을 추가하고 회원의 약관동의 흐름을 구현합니다. Term 및 MemberTerm 엔티티, 관련 리포지토리·서비스·컨트롤러·DTO를 추가하고, 회원의 약관 동의(POST /api/v1/members/term-agreements) 엔드포인트와 약관 조회(GET /api/v1/terms)를 도입합니다. Changes
Sequence DiagramsequenceDiagram
participant Client as "Client"
participant Controller as "MemberController"
participant Service as "MemberService"
participant TermSvc as "TermService"
participant Repo as "MemberTermRepository"
participant DB as "Database"
Client->>Controller: POST /api/v1/members/term-agreements\n(memberId, MemberTermsRequest)
Controller->>Service: agreeTerms(memberId, request)
Service->>Repo: existsByMemberId(memberId)
Repo->>DB: SELECT COUNT FROM member_terms WHERE member_id=?
DB-->>Repo: boolean
Repo-->>Service: exists
Service->>TermSvc: getActiveTerms()
TermSvc->>DB: SELECT * FROM terms WHERE active = true
DB-->>TermSvc: List<Term>
TermSvc-->>Service: List<Term>
Service->>Service: validate(request vs active terms)\n(check duplicates, missing, required)
Service->>Repo: saveAll(List<MemberTerm>)
Repo->>DB: INSERT INTO member_terms ...
DB-->>Repo: result
Repo-->>Service: savedEntities
Service-->>Controller: void
Controller-->>Client: 200 OK
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
src/main/java/org/sopt/kareer/global/config/swagger/SwaggerResponseDescription.java (1)
24-24: 와일드카드 import가 다른 import들과 일관성이 없습니다.파일의 다른 static import들은 모두 명시적으로 개별 에러 코드를 import하고 있는 반면,
TermErrorCode만 와일드카드(*)를 사용하고 있습니다. 일관성을 위해 명시적 import를 권장합니다.♻️ 제안하는 수정
-import static org.sopt.kareer.domain.term.exception.TermErrorCode.*; +import static org.sopt.kareer.domain.term.exception.TermErrorCode.DUPLICATE_TERM; +import static org.sopt.kareer.domain.term.exception.TermErrorCode.MISSING_TERM; +import static org.sopt.kareer.domain.term.exception.TermErrorCode.REQUIRED_TERM_NOT_AGREED;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/org/sopt/kareer/global/config/swagger/SwaggerResponseDescription.java` at line 24, The static import for TermErrorCode uses a wildcard (import static org.sopt.kareer.domain.term.exception.TermErrorCode.*;) which is inconsistent with other explicit static imports; replace the wildcard with explicit imports of the specific error constants used in SwaggerResponseDescription (e.g., import static org.sopt.kareer.domain.term.exception.TermErrorCode.SOME_ERROR; import static org.sopt.kareer.domain.term.exception.TermErrorCode.ANOTHER_ERROR;) so the file matches the explicit import style used elsewhere and references the exact TermErrorCode constants used in this class.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@src/main/java/org/sopt/kareer/domain/member/dto/request/MemberTermsRequest.java`:
- Around line 13-14: Add `@NotNull` to the list element type in MemberTermsRequest
so null entries are rejected at validation time: update the agreements field
declaration (List<@Valid TermAgreement> agreements) to include element-level
`@NotNull` (e.g., List<@NotNull `@Valid` TermAgreement> agreements) so requests like
agreements: [null] fail validation before TermAgreement::termId is accessed.
In `@src/main/java/org/sopt/kareer/domain/member/service/MemberService.java`:
- Around line 258-271: The current flow in MemberService (mapping activeTerms ->
MemberTerm.create(...) then memberTermRepository.saveAll) blindly inserts new
MemberTerm rows causing duplicate (member_id, term_id) entries; change it to
first load existing MemberTerm records for the member for all active term IDs
(use memberTermRepository to fetch by member and term IDs), then for each active
term either update the existing MemberTerm's agreed flag and timestamps or
create a new MemberTerm if none exists (use MemberTerm.create only for new
records), collect updated+new entities and call saveAll to perform upserts; also
ensure the MemberTerm entity/database has a unique constraint on (member_id,
term_id) to enforce uniqueness at DB level.
In `@src/main/java/org/sopt/kareer/domain/term/dto/response/TermsResponse.java`:
- Around line 27-28: The `@Schema` example for the boolean field required in
TermsResponse is a string and must be a boolean; update the `@Schema` annotation
on the required field in the TermsResponse DTO so its example is a boolean
literal (true or false) instead of the current string ("1. Purpose ~"), ensuring
the example type matches the field type.
---
Nitpick comments:
In
`@src/main/java/org/sopt/kareer/global/config/swagger/SwaggerResponseDescription.java`:
- Line 24: The static import for TermErrorCode uses a wildcard (import static
org.sopt.kareer.domain.term.exception.TermErrorCode.*;) which is inconsistent
with other explicit static imports; replace the wildcard with explicit imports
of the specific error constants used in SwaggerResponseDescription (e.g., import
static org.sopt.kareer.domain.term.exception.TermErrorCode.SOME_ERROR; import
static org.sopt.kareer.domain.term.exception.TermErrorCode.ANOTHER_ERROR;) so
the file matches the explicit import style used elsewhere and references the
exact TermErrorCode constants used in this class.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 571cc397-61a3-41f9-85fa-eb5af7605822
📒 Files selected for processing (15)
src/main/java/org/sopt/kareer/domain/member/controller/MemberController.javasrc/main/java/org/sopt/kareer/domain/member/dto/request/MemberTermsRequest.javasrc/main/java/org/sopt/kareer/domain/member/entity/MemberTerm.javasrc/main/java/org/sopt/kareer/domain/member/repository/MemberTermRepository.javasrc/main/java/org/sopt/kareer/domain/member/service/MemberService.javasrc/main/java/org/sopt/kareer/domain/term/controller/TermController.javasrc/main/java/org/sopt/kareer/domain/term/dto/response/TermsResponse.javasrc/main/java/org/sopt/kareer/domain/term/entity/Term.javasrc/main/java/org/sopt/kareer/domain/term/entity/enums/TermType.javasrc/main/java/org/sopt/kareer/domain/term/exception/TermErrorCode.javasrc/main/java/org/sopt/kareer/domain/term/exception/TermException.javasrc/main/java/org/sopt/kareer/domain/term/repository/TermRepository.javasrc/main/java/org/sopt/kareer/domain/term/service/TermService.javasrc/main/java/org/sopt/kareer/global/config/swagger/SwaggerResponseDescription.javasrc/main/java/org/sopt/kareer/global/security/filter/OnboardingRestrictionFilter.java
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/main/java/org/sopt/kareer/domain/term/exception/TermErrorCode.java (1)
1-1:TermErrorCode는errorcode서브패키지로 분리하는 것을 권장합니다.Line 1 기준 현재
exception패키지에 ErrorCode enum이 있어 예외 타입과 코드 정의의 책임이 섞여 보입니다.org.sopt.kareer.domain.term.errorcode로 이동하면 도메인 응집성과 탐색성이 더 좋아집니다.제안 diff
- package org.sopt.kareer.domain.term.exception; + package org.sopt.kareer.domain.term.errorcode;- public class TermException extends CustomException { - public TermException(TermErrorCode errorCode) { + import org.sopt.kareer.domain.term.errorcode.TermErrorCode; + + public class TermException extends CustomException { + public TermException(TermErrorCode errorCode) { super(errorCode); } }Based on learnings: 도메인별 ErrorCode enum은 각 도메인 패키지 내
errorcode서브패키지에 위치시켜 도메인 응집성을 높이는 것이 좋습니다.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/org/sopt/kareer/domain/term/exception/TermErrorCode.java` at line 1, Move the TermErrorCode enum out of the exception package into a new errorcode subpackage to separate error-code definitions from exception types: create package org.sopt.kareer.domain.term.errorcode, update the TermErrorCode file's package declaration accordingly, and update all references/imports to TermErrorCode in classes like any TermException implementations or handlers so they import org.sopt.kareer.domain.term.errorcode.TermErrorCode; ensure build/imports compile after the package rename.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@src/main/java/org/sopt/kareer/domain/term/exception/TermErrorCode.java`:
- Line 1: Move the TermErrorCode enum out of the exception package into a new
errorcode subpackage to separate error-code definitions from exception types:
create package org.sopt.kareer.domain.term.errorcode, update the TermErrorCode
file's package declaration accordingly, and update all references/imports to
TermErrorCode in classes like any TermException implementations or handlers so
they import org.sopt.kareer.domain.term.errorcode.TermErrorCode; ensure
build/imports compile after the package rename.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 4eb26b87-12dc-4411-8f02-652158209b5b
📒 Files selected for processing (6)
src/main/java/org/sopt/kareer/domain/member/dto/request/MemberTermsRequest.javasrc/main/java/org/sopt/kareer/domain/member/entity/MemberTerm.javasrc/main/java/org/sopt/kareer/domain/member/repository/MemberTermRepository.javasrc/main/java/org/sopt/kareer/domain/member/service/MemberService.javasrc/main/java/org/sopt/kareer/domain/term/dto/response/TermsResponse.javasrc/main/java/org/sopt/kareer/domain/term/exception/TermErrorCode.java
✅ Files skipped from review due to trivial changes (1)
- src/main/java/org/sopt/kareer/domain/term/dto/response/TermsResponse.java
🚧 Files skipped from review as they are similar to previous changes (4)
- src/main/java/org/sopt/kareer/domain/member/repository/MemberTermRepository.java
- src/main/java/org/sopt/kareer/domain/member/dto/request/MemberTermsRequest.java
- src/main/java/org/sopt/kareer/domain/member/service/MemberService.java
- src/main/java/org/sopt/kareer/domain/member/entity/MemberTerm.java
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Related issue 🛠
Work Description 📝
1. 약관 조회 API
2. 약관 동의 저장 API
에 대해 예외를 던지게 했습니다.
ScreenShots 📷
(1) 누락된 약관 동의가 있는 경우
(2) 중복된 약관 동의가 있는경우
(3) 필수 약관에 동의하지 않은 경우
(4) 이미 약관 동의 내용이 있는 경우
(5) 성공
To Reviewers 📢
Summary by CodeRabbit